Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove multiaddr dependency from libp2p-identity #3656

Merged
merged 18 commits into from
Jun 6, 2023

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Mar 22, 2023

Description

Related: multiformats/rust-multiaddr#73.
Depends-On: #3514.

Notes & open questions

Depends on a new release of multihash.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@thomaseizinger thomaseizinger added this to the v0.52.0 milestone Mar 22, 2023
identity/CHANGELOG.md Outdated Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented Mar 22, 2023

This PR also upgrades to multihash 0.18 because the next multiaddr release (which will depend on libp2p-identity 0.2.0) will also depend on multihash 0.18.

See multiformats/rust-multiaddr#83.

@thomaseizinger thomaseizinger requested a review from mxinden March 22, 2023 15:30
@thomaseizinger
Copy link
Contributor Author

I'd like to release libp2p-identity 0.2.0 from this branch so we can push multiformats/rust-multiaddr#83 forward.

@thomaseizinger
Copy link
Contributor Author

I'd like to release libp2p-identity 0.2.0 from this branch so we can push multiformats/rust-multiaddr#83 forward.

In out-of-band meeting with @mxinden, we decided to not do this because the release tag would get lost in our Git history due to our squash-merge strategy.

@mergify

This comment was marked as resolved.

@mergify

This comment was marked as resolved.

@mergify

This comment was marked as resolved.

@mergify

This comment was marked as resolved.

@mergify

This comment was marked as resolved.

@mergify

This comment was marked as resolved.

@mergify
Copy link
Contributor

mergify bot commented May 4, 2023

This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏

@mxinden
Copy link
Member

mxinden commented May 8, 2023

Please ping me once this is ready for a review.

@thomaseizinger
Copy link
Contributor Author

Please ping me once this is ready for a review.

Will do!

The order of things for this to happen is documented here: multiformats/rust-multiaddr#83 (comment)

@thomaseizinger thomaseizinger marked this pull request as ready for review June 6, 2023 11:04
identity/src/peer_id.rs Outdated Show resolved Hide resolved
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case my latest comment is correct and rust-multihash is released, feel free to merge here and release libp2p-identity v0.2.0.

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented Jun 6, 2023

Merging here based on @mxinden's review above. Worst case, the function can be added again in a patch-release.

@mergify mergify bot merged commit 0db9937 into master Jun 6, 2023
@mergify mergify bot deleted the libp2p-identity-no-multiaddr branch June 6, 2023 17:27
mergify bot pushed a commit that referenced this pull request Jun 8, 2023
This updates `multiaddr` to version `0.19`.

Depends-On: #3656.
Depends-On: multiformats/rust-multiaddr#83.
Resolves: #4039.

Pull-Request: #4037.
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
This updates `multiaddr` to version `0.19`.

Depends-On: libp2p#3656.
Depends-On: multiformats/rust-multiaddr#83.
Resolves: libp2p#4039.

Pull-Request: libp2p#4037.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants